fix: Handle invalid Extension Settings#37
Conversation
| function extractRoktExtensions(settingsString) { | ||
| var settings = settingsString ? parseSettingsString(settingsString) : []; | ||
|
|
||
| var settings = parseSettingsString(settingsString); |
There was a problem hiding this comment.
I think this should still have the conditional. otherwise everyone immediately will start to see a console.error about there not being invalid json right?, right?
There was a problem hiding this comment.
Pull Request Overview
This PR improves resilience against invalid extension settings by logging parse errors and returning empty lists instead of throwing. It also adds tests to verify behavior for malformed inputs.
- Changed
parseSettingsStringto catch JSON errors, log them, and return an empty array. - Updated
extractRoktExtensionsto handleundefinedandnullsettings without throwing. - Added unit tests covering invalid setting strings.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/src/tests.js | Added test cases for NONE, undefined, and null inputs. |
| src/Rokt-Kit.js | Modified error handling in parseSettingsString and maintained loop logic in extractRoktExtensions. |
Comments suppressed due to low confidence (2)
test/src/tests.js:1286
- Consider adding a test for an empty string (
'') input to cover another common invalid case.
.extractRoktExtensions('NONE')
test/src/tests.js:1284
- It may be valuable to assert that
console.erroris called when parsing invalid JSON to ensure the error logging path is covered.
it('should handle invalid setting strings', () => {
| return JSON.parse(settingsString.replace(/"/g, '"')); | ||
| } catch (error) { | ||
| throw new Error('Settings string contains invalid JSON'); | ||
| console.error('Settings string contains invalid JSON'); |
There was a problem hiding this comment.
[nitpick] Consider including the original error object when logging JSON parse failures to provide more context, e.g., console.error('Settings string contains invalid JSON', error);.
| console.error('Settings string contains invalid JSON'); | |
| console.error('Settings string contains invalid JSON', error); |
Summary
NONEorNULL.Testing Plan
roktExtensionsettings.